Extract early config validation from runServeCmd into testable helpers by raju249 · Pull Request #45583 · fleetdm/fleet

raju249 · 2026-05-15T12:47:02Z

Extracts five pure config validations from runServeCmd into helpers in a new cmd/fleet/serve_validation.go, and adds table-driven unit tests for each:

validateOTELLoggingConfig — OTEL logs requires tracing enabled
validateOsqueryHostIdentifier — must be one of provided, instance, uuid, hostname
normalizeAndValidateServerURLPrefix — trims/normalizes leading-slash, validates against allowedURLPrefixRegexp
validateServerPrivateKeyExclusive — private_key and private_key_arn are mutually exclusive
validateServerPrivateKeyLength — minimum 32 bytes when set

Behavior is byte-identical: error messages preserved, runServeCmd still calls initFatal with the same descriptions at the same points. serve.go shrinks by 21 lines.

Broader plan

Issue #33370 calls for moving logic out of serve.go ("should only contain critical config and dependency injection logic"). This PR is one slice. Realistic follow-ups, each in their own small PR:

Extract more config validations (Apple APNs/SCEP both-or-neither, etc.)
Use the initFatal injection from Inject initFatal as a package-level var (#33370) #45343 to cover runtime failure paths (datastore init, Redis init, MDM init, etc.)
Larger extractions (license init, MDM wiring, mailer init)

Related issue: Refs #33370

Checklist for submitter

Added/updated automated tests
Input data is properly validated (helpers added, no SQL/JS/shell paths involved)
Changes file: not applicable — internal refactor with no user-visible behavior change

Summary by CodeRabbit

Refactor
- Improved server startup configuration validation with modular helper functions for better maintainability.
Tests
- Added comprehensive unit tests for server configuration validation logic, covering private key settings, osquery identifiers, OTEL logging configuration, and URL prefix normalization.

Move five pure config validations (OTEL logs/tracing dependency, osquery host identifier, server URL prefix normalization, private key flag exclusivity, private key length) into helper functions in cmd/fleet/serve_validation.go and add unit tests for each. Behavior is unchanged; runServeCmd still fatals at the same points via the same initFatal descriptions. Part of fleetdm#33370.

claude

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

coderabbitai · 2026-05-15T12:56:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 230a2548-b5f4-4cb0-80a8-ba47fa3f4e34

📥 Commits

Reviewing files that changed from the base of the PR and between 2c47f8e and eb459f6.

📒 Files selected for processing (3)

cmd/fleet/serve.go
cmd/fleet/serve_validation.go
cmd/fleet/serve_validation_test.go

Walkthrough

This PR consolidates server startup configuration validation by extracting inline validation logic from cmd/fleet/serve.go into dedicated helper functions in a new file cmd/fleet/serve_validation.go. Five validation helpers enforce server private key constraints (mutual exclusivity and minimum length), restrict osquery host identifier to an allowlist, validate OTEL logging dependencies, and normalize/validate server URL prefix. The refactored serve.go delegates to these helpers at three validation checkpoints, and comprehensive table-driven tests cover all validation scenarios and edge cases.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: extracting configuration validation logic from runServeCmd into testable helper functions.
Description check	✅ Passed	The description covers the five validations extracted, explains behavior is identical, notes the shrinking of serve.go, references the related issue, and marks the applicability of checklist items appropriately.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

raju249 · 2026-05-15T13:00:32Z

@getvictor - I did some more refactoring of serve.go file. Can you please take a review?

Thanks! 🙏 🙇

codecov · 2026-05-15T13:07:30Z

Codecov Report

❌ Patch coverage is 75.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.73%. Comparing base (c19df6d) to head (eb459f6).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fleet/serve.go	0.00%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #45583      +/-   ##
==========================================
- Coverage   66.74%   66.73%   -0.01%     
==========================================
  Files        2740     2741       +1     
  Lines      219163   219178      +15     
  Branches    10947    10947              
==========================================
- Hits       146283   146277       -6     
- Misses      59649    59677      +28     
+ Partials    13231    13224       -7

Flag	Coverage Δ
backend	`68.58% <75.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

raju249 requested a review from a team as a code owner May 15, 2026 12:47

claude Bot reviewed May 15, 2026

View reviewed changes

getvictor self-assigned this May 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract early config validation from runServeCmd into testable helpers#45583

Extract early config validation from runServeCmd into testable helpers#45583
raju249 wants to merge 1 commit into
fleetdm:mainfrom
raju249:33370-serve-config-validation-tests

raju249 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

claude Bot left a comment

Uh oh!

coderabbitai Bot commented May 15, 2026

❌ Failed checks (1 warning)

Uh oh!

raju249 commented May 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

raju249 commented May 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Broader plan

Checklist for submitter

Summary by CodeRabbit

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai Bot commented May 15, 2026

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

raju249 commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 15, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

raju249 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

raju249 commented May 15, 2026 •

edited

Loading